Avoid duplicate temperature compensation in EAHRS by ohyaiamhere · Pull Request #30863 · ArduPilot/ardupilot

ohyaiamhere · 2025-08-08T13:30:56Z

As explained in issue #25792, there is an issue where EAHRS performs duplicate temperature compensation for certain sensors.

The enum TempCalibration of the type TempCal is adjusted to see if the sensors is of the MicroStrain7 type (as mentioned in the issue) and set the enum as IsTempCalibrated. Else the enum is set as IsNotTempCalibrated.

In the inertial sensor section, this enum, which was sent by the message ins_data_message_t, is checked, and if there is temperature compensation, a new bool IsTempCalibrated is set to true. This bool then checked at places where temperature compensation due to HAL_INS_TEMPERATURE_CAL_ENABLE occurs and the code inside is executed if the bool is false.

peterbarker

Why are the temperature calibration values non-zero for the external device?

ohyaiamhere · 2025-10-22T12:41:13Z

Why are the temperature calibration values non-zero for the external device?

Hello Peter,

The function I had created earlier was not correctly attaching the TempCal data with the ins_data_message_t

I have modified the commit to deal with this issue.

peterbarker

This is only a partial review - just want to make sure I know what you're trying to do here.

So I think what you're trying to do here is cope with the fact that Microstrain can supply IMU data which has already been adjusted for the temperatures in which the microstrain is operating?

... and that thus you do not want to learn any offsets from the microstrain when we are learning?

Thing is that I'm pretty sure there are many dragons with using tcal and externalahrs IMU data.

I'm particularly concerned about what happens if you disable the IMU data from the externalahrs - do we start to apply the learnt coefficients to the wrong IMU now?!

peterbarker

OK, so this is much simpler than it was :-)

We'll need a report on what testing has been done for this PR.

peterbarker · 2025-11-19T23:20:40Z

+        AP_ExternalAHRS::ins_data_message_t ins;
+        ins.accel = static_cast<Vector3f>(imu_data.accel);
+        ins.gyro = static_cast<Vector3f>(imu_data.gyro);
+        ins.temperature = static_cast<float>(-300);


.... so you still haven't put this back into the structure... and these casts weren't required before when using the structure.... I think you stopped adding something in here, so perhaps just remove this patch as it doesn't seem to be doing anything.

peterbarker · 2025-11-19T23:21:09Z

+        ins.accel = static_cast<Vector3f>(imu_data.accel);
+        ins.gyro = static_cast<Vector3f>(imu_data.gyro);
+        ins.temperature = static_cast<float>(-300);
+        ins.TempCalibration = static_cast<AP_ExternalAHRS::TempCal>(AP_ExternalAHRS::TempCal::IsTempCalibrated);


Missing the point. Continue to use the structure, cast only what is required when putting data into the structure.

typedef struct { Vector3f accel; Vector3f gyro; float temperature; TempCal TempCalibration = TempCal::IsNotTempCalibrated; } ins_data_message_t;

Due to this modification being added in AP_ExternalAHRS.h the structure cannot be reused in either of the two files. Each time I try to, it throws an error message.

../../libraries/AP_ExternalAHRS/AP_ExternalAHRS_MicroStrain5.cpp: In member function ‘void AP_ExternalAHRS_MicroStrain5::post_imu() const’: ../../libraries/AP_ExternalAHRS/AP_ExternalAHRS_MicroStrain5.cpp:131:9: error: no matching function for call to ‘AP_ExternalAHRS::ins_data_message_t::ins_data_message_t(<brace-enclosed initializer list>)’ 131 | };

So another option would be to figure out how to set this default value using another method, and changing this line to

TempCal TempCalibration;

I notice this only in the two MicroStrain drivers but not the others because they use direct assignment, as I had used in the patch before this new one. Like the InertialLabs driver below

state.accel = ins_data.accel; state.gyro = ins_data.gyro;

ohyaiamhere · 2025-11-21T14:14:40Z

OK, so this is much simpler than it was :-)

We'll need a report on what testing has been done for this PR.

I have explained what I have done below:

Implementation logic

The enum, called TempCal is carried in the ins_data_message_t, from the backend of the AP_ExternalAHRS and the drivers of the individual devices, to AP_InertialSensor. where it is received in pkt

The value of the enum is then used to change the value of tcal(i).enable to AP_InertialSensor_TCal::Enable::Disabled for that instance i of the sensor. This is done to make sure that, whenever this packet is received, the autopilot turns off the temperature calibration for that particular instance of sensor. This implementation is cleaner than creating another variable
externally_temperature_calibrated.

Testing

Using units tests with GNU Debugger, the value of the incoming packet was checked to make sure that the correct value of TempCal was being carried for the device which is already calibrated, and the device which isn't.

For MicroStrain 5
./Tools/autotest/autotest.py --debug --gdb build.Plane -B AP_InertialSensor::handle_external test.Plane.MicroStrainEAHRS5

For MicroStrain 7
./Tools/autotest/autotest.py --debug --gdb build.Plane -B AP_InertialSensor::handle_external test.Plane.MicroStrainEAHRS7

Please let me know what you think, and how else can this be tested. I do not have access to hardware myself, so SITL testing is all I am able to do.

EDIT: Changed the assignment of tcal(i).enable to before the recursive call instead of after.

tolesam · 2026-02-11T09:09:57Z

@ohyaiamhere Can you add it for SBG driver as well please? SBG's INS are always calibrated in temperature.

ohyaiamhere · 2026-02-11T12:03:55Z

@ohyaiamhere Can you add it for SBG driver as well please? SBG's INS are always calibrated in temperature.

I can create a commit that adds a cached.sensors.ins_data.TempCalibration around line 633 in the file AP_ExternalAHRS_SBG.cpp

    if (updated_ins) {
        cached.sensors.ins_data.accel = state.accel;
        cached.sensors.ins_data.gyro = state.gyro;
        cached.sensors.ins_ms = now_ms;
        AP::ins().handle_external(cached.sensors.ins_data);
    }

where ins_data is updated.

Although I couldn't find a unit test for this sensor and I do not have the hardware to test it myself, I can make another commit in this pull request. Let me know what you think @peterbarker .

tolesam · 2026-02-11T13:53:01Z

@ohyaiamhere Can you add it for SBG driver as well please? SBG's INS are always calibrated in temperature.

I can create a commit that adds a cached.sensors.ins_data.TempCalibration around line 633 in the file AP_ExternalAHRS_SBG.cpp
    if (updated_ins) {
        cached.sensors.ins_data.accel = state.accel;
        cached.sensors.ins_data.gyro = state.gyro;
        cached.sensors.ins_ms = now_ms;
        AP::ins().handle_external(cached.sensors.ins_data);
    }
where ins_data is updated.

Although I couldn't find a unit test for this sensor and I do not have the hardware to test it myself, I can make another commit in this pull request. Let me know what you think @peterbarker .

That sounds good to me.
You're right for the unit test missing, I have to implement it.

ohyaiamhere · 2026-02-11T14:40:37Z

@ohyaiamhere Can you add it for SBG driver as well please? SBG's INS are always calibrated in temperature.

I can create a commit that adds a cached.sensors.ins_data.TempCalibration around line 633 in the file AP_ExternalAHRS_SBG.cpp
    if (updated_ins) {
        cached.sensors.ins_data.accel = state.accel;
        cached.sensors.ins_data.gyro = state.gyro;
        cached.sensors.ins_ms = now_ms;
        AP::ins().handle_external(cached.sensors.ins_data);
    }
where ins_data is updated.
Although I couldn't find a unit test for this sensor and I do not have the hardware to test it myself, I can make another commit in this pull request. Let me know what you think @peterbarker .
That sounds good to me. You're right for the unit test missing, I have to implement it.

I have created the commit. If you have the hardware, would it be possible for you to help test it?

…Microstrain7

…_data_message_t

ohyaiamhere changed the title ~~Avoid duplicate temperature compensation in EHRS~~ Avoid duplicate temperature compensation in EAHRS Aug 8, 2025

ohyaiamhere force-pushed the temp_cal branch 4 times, most recently from 34370d4 to 67075aa Compare August 13, 2025 15:45

ohyaiamhere force-pushed the temp_cal branch 4 times, most recently from 3f98453 to 9a97e14 Compare August 25, 2025 10:34

Ryanf55 mentioned this pull request Aug 25, 2025

EAHRS is performing duplicate temperature compenstation #25792

Open

ohyaiamhere force-pushed the temp_cal branch 3 times, most recently from e994bc3 to 9ff73df Compare September 1, 2025 06:59

ohyaiamhere force-pushed the temp_cal branch from 9ff73df to 0f88775 Compare September 5, 2025 14:38

ohyaiamhere force-pushed the temp_cal branch 2 times, most recently from 1afaa03 to bfa153d Compare September 30, 2025 13:26

ohyaiamhere force-pushed the temp_cal branch from bfa153d to a5e388a Compare October 18, 2025 07:46

peterbarker reviewed Oct 19, 2025

View reviewed changes

ohyaiamhere force-pushed the temp_cal branch 4 times, most recently from 96976bb to edcb31b Compare October 21, 2025 21:11

ohyaiamhere force-pushed the temp_cal branch from edcb31b to ae7f14f Compare October 22, 2025 12:41

peterbarker requested changes Oct 31, 2025

View reviewed changes

ohyaiamhere force-pushed the temp_cal branch 5 times, most recently from 3059ee9 to dd16037 Compare November 5, 2025 14:18

ohyaiamhere force-pushed the temp_cal branch 9 times, most recently from 618f891 to a56db9b Compare November 19, 2025 15:30

ohyaiamhere marked this pull request as ready for review November 19, 2025 15:38

peterbarker requested changes Nov 19, 2025

View reviewed changes

ohyaiamhere marked this pull request as draft November 20, 2025 08:03

ohyaiamhere force-pushed the temp_cal branch 9 times, most recently from fcee1fd to 957b558 Compare November 21, 2025 13:59

peterbarker mentioned this pull request Feb 6, 2026

AP_ExternalAHRS: Fix duplicate barometer temperature compensation #32113

Closed

ohyaiamhere added 3 commits March 10, 2026 15:30

AP_InertialSensor: Added hook to read the temp compensation enum for …

e4eb2c3

…Microstrain7

AP_ExternalAHRS: Added enum for temperature calibration status to ins…

5a1fe91

…_data_message_t

AP_InertialSensor: Added hook to read the temp compensation enum for SBG

2c85474

Conversation

ohyaiamhere commented Aug 8, 2025

Uh oh!

peterbarker left a comment

Choose a reason for hiding this comment

Uh oh!

ohyaiamhere commented Oct 22, 2025

Uh oh!

peterbarker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

peterbarker left a comment

Choose a reason for hiding this comment

Uh oh!

peterbarker Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

peterbarker Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

ohyaiamhere Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ohyaiamhere commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tolesam commented Feb 11, 2026

Uh oh!

ohyaiamhere commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tolesam commented Feb 11, 2026

Uh oh!

ohyaiamhere commented Feb 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ohyaiamhere Nov 20, 2025 •

edited

Loading

ohyaiamhere commented Nov 21, 2025 •

edited

Loading

ohyaiamhere commented Feb 11, 2026 •

edited

Loading